Claude finished @dario-daza's task in 2m 32s —— View job

Rollback Safety Analysis

• Read rollback-unsafe categories reference
• Get full PR diff
• Analyze diff against all categories
• Post findings and apply labels

Pull Request Unsafe to Rollback!!!

• Category: M-3 — REST / GraphQL / Headless API Contract Change

• Risk Level: 🟡 MEDIUM

• Why it's unsafe: This PR adds a new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint and updates the Angular frontend to call it (replacing the previous PUT /v1/contenttype/id/{idOrVar} call). If the backend is rolled back to N-1 while a browser or CDN has cached the N-version Angular frontend, any attempt to save a Style Editor Schema will hit the new PATCH endpoint which does not exist on N-1, resulting in a 404 error. The frontend does include a 404 fallback (#putMetadataFallback) that retries with a full PUT, which mitigates the rollback risk significantly — but the fallback relies on the Angular bundle itself being current, and any error in the fallback path still degrades the user experience.

  Additionally, the behavior of the existing PUT /v1/contenttype/id/{idOrVar} endpoint has been semantically changed: previously, omitting metadata from the request body removed it (fully destructive); now, omitting metadata preserves the existing server-side metadata. Any N-1 client that relied on the old destructive metadata semantics (e.g., clearing metadata by omission) will observe unexpected behavior if that client calls the N-deployed backend before rollback is complete.

• Code that makes it unsafe:

  • dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java — new @PATCH @Path("/id/{idOrVar}/metadata") handler added; also putContentTypeUpdate now calls contentTypeHelper.preserveMetadataIfAbsent(...) changing the semantics of the existing PUT
  • dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java — preserveMetadataIfAbsent method (line ~1100) changes PUT endpoint contract: absent metadata key now preserves server-side metadata instead of clearing it
  • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.ts — the save() method now calls PATCH /api/v1/contenttype/id/${contentType.id}/metadata instead of the old PUT endpoint; a 404 fallback is present but adds fragility
  • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — new path /v1/contenttype/id/{idOrVar}/metadata (patch) added; PUT description updated to reflect the new metadata preservation semantics

• Alternative (if possible): The risk is meaningfully reduced by the 404 fallback in the frontend. The remaining risk is bounded to: (a) clients other than the Angular frontend that relied on the old metadata-clearing-by-omission behavior of the PUT endpoint, and (b) any edge case in the fallback path. Since the old PUT endpoint is still present and functional, operators can restore full functionality after rollback by clearing CDN/browser caches. No database migration or reindex is involved — this is a pure API contract change.

Pull Request Unsafe to Rollback!!!

• Category: M-3 — REST / GraphQL / Headless API Contract Change
• Risk Level: 🟡 MEDIUM
• Why it's unsafe: This PR adds a new PATCH /v1/contenttype/id/{idOrVar}/metadata endpoint and updates the Angular frontend component in the same PR to call it (replacing the previous PUT /v1/contenttype/id/{idOrVar} call). If the backend is rolled back to N-1 while a browser or CDN has cached the N-version Angular frontend, any attempt to save a Style Editor Schema will call the new endpoint which does not exist on N-1, resulting in a 404 error. The user will be unable to save Style Editor Schemas until the browser cache clears.
• Code that makes it unsafe:
  • dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java — new @PATCH @Path("/id/{idOrVar}/metadata") handler added at line 2022+
  • core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.ts — line 247: this.#http.patch\<DotCMSResponse\>(v1/contenttype/id/${contentType.id}/metadata, metadataPatch) replaces the prior DotCrudService.putData call to the old PUT endpoint
  • dotCMS/src/main/webapp/WEB-INF/openapi/openapi.yaml — new path /v1/contenttype/id/{idOrVar}/metadata added (lines 8148+)
• Alternative (if possible): Since the new endpoint is purely additive (a new path, no existing path removed or changed), the rollback risk is bounded: only the Style Editor Schema save fails with 404, and only while the N-frontend is cached. If the old PUT /v1/contenttype/id/{idOrVar} endpoint is preserved (which it is — this PR does not remove it), operators can restore full functionality by clearing the CDN cache and/or forcing a frontend refresh after rollback.

❌ Codex Review failed — openai.gpt-5.5

The review job failed before producing output. See the run for details.

_{Run: #27579601571}

Question: is metadata preserved when a Content Type is saved through the regular full PUT?

This new endpoint (PATCH /api/v1/contenttype/id/{idOrVar}/metadata) merges into the existing metadata and serializes DOT_STYLE_EDITOR_SCHEMA correctly — that part looks good.

My concern is the interaction with the regular update path (putContentTypeUpdate → PUT /api/v1/contenttype/id/{idOrVar}), whose Swagger description states:

⚠️ Destructive semantics. This endpoint treats the request body as the full desired state. Any editable property (including items in fields[] and metadata keys) absent from the body will be removed.

So if any client does a normal full-CT PUT without carrying DOT_STYLE_EDITOR_SCHEMA in metadata, the schema written by this new endpoint would be silently wiped.

1. Does the CT edit screen (or any existing client) round-trip the full metadata map — including DOT_STYLE_EDITOR_SCHEMA — on a regular save? If not, a normal save would clobber the style editor schema.
2. Should the style editor schema be protected against the full-PUT's destructive merge (e.g. preserved server-side unless explicitly cleared), or is the contract strictly "GET → mutate → PUT the whole object"?

Mainly want to make sure the schema persisted here survives a subsequent normal CT save.

The way we handled it in the frontend, it never happens, but it is possible that any REST API caller (Postman, a migration script, or a third-party integration) that makes a raw PUT and omits metadata from the body would silently wipe the schema. I added a validation that prevents this and preserves the metadata if a user omits it in a PUT request. If you want to wipe the metadata, you'll need to send metadata: null.

🤖 Bedrock Review — deepseek.v3.2

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1026 — Missing @WrapInTransaction on mergeAndSaveMetadata. This method performs a read-modify-write sequence (contentTypeAPI.find → contentTypeAPI.save) and should be transactional to ensure data integrity. Without it, partial failures could leave metadata in an inconsistent state.

[🟠 High] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1037 — Missing permission check before editing content type. The method calls contentTypeAPI.save without verifying the user has EDIT permission on the content type. Should call contentTypeAPI.find with RESPECT_FRONTEND_ROLES = false and check permissions, or rely on the API's internal check (which may not throw DotSecurityException for all unauthorized cases). This could allow unauthorized metadata updates.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1050 — Catch-all Throwable may obscure real errors. The catch (final Throwable t) wraps everything in a generic DotDataException, which could hide specific exceptions like DotSecurityException or BadRequestException, making debugging harder. Should rethrow known exceptions (DotDataException, DotSecurityException, BadRequestException) and only wrap unexpected Throwables.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1080 — Logging a warning inside normalizeStyleEditorSchemaToString could be noisy in production if clients frequently send non‑string values. Consider lowering to DEBUG or moving the log to only when serialization fails (already caught by the try block).

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1128 — preserveMetadataIfAbsent may discard fields if existing.metadata() is null. When existing.metadata() is null, the method returns the original contentType, but the fields are already attached via constructWithFields earlier. This is fine, but the comment "re‑attaches the field list" is misleading when metadata is preserved; fields are already present.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeHelper.java:1155 — requestContainsKey parses JSON via toString() on arbitrary Object requestJson. If requestJson is not a string representation of JSON (e.g., a Map), toString() may produce malformed JSON causing parse failure and a false negative. Should handle Map/Collection types directly or use Jackson's convertValue.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java:2028 — Missing validation for metadataPatch parameter. The method accepts Map<String, Object> but does not validate that values are JSON‑serializable (aside from DOT_STYLE_EDITOR_SCHEMA normalization). Malformed values could cause downstream serialization errors.

[🟡 Medium] dotCMS/src/main/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResource.java:2045 — Returning HTTP 200 with existing content type when metadataPatch is empty/null is a deviation from typical PATCH semantics (no‑op should still return 200, but returning the full entity is fine). However, logging a warning for an empty patch could clutter logs. Consider changing to DEBUG.

[🟡 Medium] core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.ts:219 — Hardcoded /api/v1/contenttype/id/${contentType.id}/metadata URL. Should use a service or constant to avoid duplication if the endpoint path changes.

[🟡 Medium] core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.ts:229 — N‑1 fallback logic (err.status === 404) assumes any 404 means the PATCH endpoint is missing. A 404 could also mean the content type was deleted between the GET and PATCH. Should distinguish by checking error response body or use a more specific condition (e.g., pattern match on error message).

[🟡 Medium] core-web/apps/dotcms-ui/src/app/portlets/shared/dot-content-types-edit/components/style-editor/dot-style-editor-builder.component.ts:267 — #putMetadataFallback sends a full PUT with workflow: contentType.workflows.map((w) => w.id). This assumes workflows array exists and contains objects with id. If workflows is undefined or malformed, this will throw. Should guard with optional chaining/default.

[🟠 High] dotcms-integration/src/test/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResourceUpdateMetadataTest.java:224 — Test uses TestUserUtils.getChrisPublisherUser() which may not have edit permission, but the test expects a 403. However, the mocked WebResource bypasses real permission checks; the test may pass even if the permission logic is broken. Should verify that the real permission system is invoked (e.g., by mocking ContentTypeAPI).

[🟡 Medium] dotcms-integration/src/test/java/com/dotcms/rest/api/v1/contenttype/ContentTypeResourceUpdateMetadataTest.java:383 — Concurrency test uses CountDownLatch to maximize race condition, but the striped lock should prevent lost updates. However, the test does not verify that the lock is actually acquired (no lock monitoring). It only asserts final state, which could pass even if locks are ineffective (e.g., if both threads run sequentially). Consider adding lock‑acquisition logging or verifying concurrent execution.

[🟡 Medium] dotcms-postman/src/main/resources/postman/Define_Contentlets_StyleProperties.postman_collection.json:2475 — Postman test uses {{styleSchema}} variable which is a JSON object stringified. If the variable is not set, the request body becomes {{styleSchema}} literal, causing a server error. Should have a pre‑request check that the variable exists.

_{Run: #27662010592 · tokens: in: 19231 · out: 1448 · total: 20679}

Legal Risk

The following dependencies were released under a license that
has been flagged by your organization for consideration.

Recommendation

While merging is not directly blocked, it's best to pause and consider what it means to use this license before continuing. If you are unsure, reach out to your security team or Semgrep admin to address this issue.

GPL-2.0

• node-forge 1.4.0
• node-forge 1.3.1

MPL-2.0

• axe-core 4.11.2
• dompurify 3.3.3
• lightningcss-android-arm64 1.31.1
• lightningcss-darwin-arm64 1.31.1
• lightningcss-darwin-x64 1.31.1
• lightningcss-freebsd-x64 1.31.1
• lightningcss-linux-arm-gnueabihf 1.31.1
• lightningcss-linux-arm64-gnu 1.31.1
• lightningcss-linux-arm64-musl 1.31.1
• lightningcss-linux-x64-gnu 1.31.1
• lightningcss-linux-x64-musl 1.31.1
• lightningcss-win32-arm64-msvc 1.31.1
• lightningcss-win32-x64-msvc 1.31.1
• lightningcss 1.31.1